Cc @jameskoster @WordPress/gutenberg-core

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: afercia <[email protected]>
Co-authored-by: jameskoster <[email protected]>
Co-authored-by: jasmussen <[email protected]>
Co-authored-by: fcoveram <[email protected]>
Co-authored-by: paaljoachim <[email protected]>
Co-authored-by: SaxonF <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Size Change: +283 B (+0.02%)

Total Size: 1.84 MB

_{compressed-size-action}

I can't comment on the code but appreciate the effort to tidy this up. The modal already looks much better.

One drawback I noticed is that when there are many changes (or the viewport is small, or a combination thereof) the buttons disappear until you scroll:

There's probably a broader question to ask about whether actions should be sticky across all modals, but I mention it here because despite the other improvements this is a slight downgrade in the UX.

the buttons disappear until you scroll:

Yes, that's a feature / limitation of the Modal component. A potential change to introduce a sticky footer should be considered at the component's level.
But
I'd warn against sticky headers / footers. They come with their problems, for example they tend to hide content when navigating focusable elements with the keyboard unless some smart scroll-padding mechanism is in place.

@jameskoster any other style refining you would like to see implemented?

Flaky tests detected in fb435f8.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/12273581708
📝 Reported issues:

• [Flaky Test] Should insert content using the global inserter #57525 in /test/e2e/specs/widgets/editing-widgets.spec.js

Agree that's a detail to work out at the component level, and I don't suppose it needs to be a blocker, though I'd welcome feedback from @WordPress/gutenberg-design on that detail.

There's something about the overall spacing that works less well in this context. I wonder if it would be worth trying the small modal variant, so that the UI more closely resembles the one we find in the site editor.

Save modal	Save panel

What do you think?

What do you think?

It does seem a bit loose on the spacing, but it's unclear that needs to be a blocker. Probably worth getting this into the plugin, and gathering feedback through that channel, and then spacing polish can happen separately at any time, whether locally or component level.

Re: the small size, this is how it would look in the site editor and theme preview, on a large screen:

spacing polish can happen separately at any time

It's a one-line change, I'd agree it can happen at any time if need be.
If no other objections, I'd appreciate a review to unblock this PR. Cc @WordPress/gutenberg-core

For me the small modal works better, just because it feels more like the UI we're familiar with. Not a blocker though. As long as we use one of the defined modal sizes I'm happy.

For me the small modal works better, just because it feels more like the UI we're familiar with. Not a blocker though. As long as we use one of the defined modal sizes I'm happy.

Sure. In the latest commit I made the modal dialog small so that everyone can try it and see how it looks on different viewport sizes. Cc @WordPress/gutenberg-design

I agree with keeping the same spacing approach consistent with its version in the site editor.

Regarding follow-up improvements, I see room for improvement in adding an indentation level in the bullet points and reducing the spacing between the checkbox and the bullet list.

If I understand this correctly, more checkboxes can show up and placing the bullet list at the control label could improve the text relationship between what users are about to update and what that includes.

This suggestion is for both modal and the inspector panel.

Good point about the indentation. I'd like to see the list items text aligned with the checkbox label, with the bullets following the principle of the hanging punctuation:

Latest commit adjusts the top and left margin of the changes list:

Since we're discussing refinements, I have two questions:

1. Why the changes list items use a so small font size? It's 12 pixels. It's very small and I would like to see the 12 pixels size avoided as much as possible.
2. Why their color is a lighter grey? I wouldn't say this information is less important to deserve a lighter grey to be less prominent. On the contrary, it's an important information that should be as readable as possible. If no objections, I would use a darker grey.

The screenshot looks good to me 👍

…following the principle of the hanging punctuation

Wow. TIL there was a word for that alignment. Thanks

Sorry. My previous comment was on this message. It seems we pressed the "comment" button almost at the same time.

Regarding your suggestions for font size and text color, I second your rationale and agree with the changes.

Latest commit uses the default font size and color for the changes list items:

Not a strong feeling but I would prefer to consider the size and color of the styles change list in a separate issue/PR. Currently the differences create a sense of hierarchy, where on this PR the panel title and change list blend together.

Also because this PR is important an enhancement, so would be good to keep focussed on the original problem.